-
Notifications
You must be signed in to change notification settings - Fork 924
fix(wasix): Prevent recursive merge fallback when mounting packages #5799
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(wasix): Prevent recursive merge fallback when mounting packages #5799
Conversation
93939e0 to
d33ca98
Compare
b4ac5d3 to
63d0b61
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the virtual filesystem handling in wasix to prevent recursive merge fallback operations when mounting packages. The core improvement is the introduction of an Overlay variant to WasiFsRoot that enables direct merging of UnionFileSystem instances without falling back to expensive recursive filesystem copies.
- Changed filesystem type handling from
BoxtoArcthroughout the codebase for better sharing semantics - Introduced
WasiFsRoot::Overlayvariant to enable efficient filesystem merging usingUnionFileSystem::merge()instead of the recursivemerge_filesystems()fallback - Extracted
RelativeOrAbsolutePathHackto its own module for better code organization
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/wasix/src/state/builder.rs | Updated set_fs() to accept impl Into<Arc<...>> instead of Box, added internal set_fs_root() method, removed Arc wrapper from sandbox_fs() since TmpFileSystem is Clone |
| lib/wasix/src/state/env.rs | Added handling for new WasiFsRoot::Overlay variant when injecting package commands |
| lib/wasix/src/fs/mod.rs | Refactored WasiFsRoot enum to include Overlay variant, updated conditional_union() to use UnionFileSystem::merge() for overlay case, changed default_fs_backing() return type to Arc |
| lib/wasix/src/fs/relative_path_hack.rs | New module containing extracted RelativeOrAbsolutePathHack implementation |
| lib/wasix/src/runtime/package_loader/load_package_tree.rs | Changed filesystem() to return Option<UnionFileSystem> instead of boxed trait object, removed empty OverlayFileSystem creation, added webc version validation |
| lib/wasix/src/runners/wasi_common.rs | Updated prepare_filesystem() to return WasiFsRoot and create Overlay variant, moved RelativeOrAbsolutePathHack to separate module |
| lib/wasix/src/runners/wasi.rs | Updated to use duplicate() method when cloning filesystem |
| lib/wasix/src/runners/wcgi/runner.rs | Updated to use duplicate() method when cloning filesystem |
| lib/wasix/src/bin_factory/binary_package.rs | Changed webc_fs field from Arc<dyn FileSystem> to Option<Arc<UnionFileSystem>> for more specific typing |
| lib/virtual-fs/src/union_fs.rs | Added merge() and duplicate() methods, made MountPoint cloneable |
| lib/virtual-fs/src/passthru_fs.rs | Updated to use Arc instead of Box for inner filesystem |
| lib/c-api/src/wasm_c_api/wasi/mod.rs | Added explicit type annotation for filesystem initialization |
| tests/lib/wast/src/wasi_wast.rs | Updated test utilities to use Arc instead of Box for filesystem types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Store the inner FS as an arc to make it more easily shareable. Preserves constructor backwards compatibility.
Add the ability to merge a union fs into another using a given merge mode.
ee8f735 to
55cda66
Compare
55cda66 to
05cd3ab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Previously adding additional dependencies to a WASI environment would lead to a recursive file system merge, which would clone all files in memory, which is very expensive. This was caused by a very unstructed creation and extension logic in the WasiFs, which could take on a complicated form of <dyn Filesystem>. There previosly were attempts to downcast and extend without merging, but those would fail in common use cases. This is fixed by adding a dedicated WasiFs::Overlay variant, which is a dedicated representation for the common wasix package environment use case. This allows to deterministically extend an existing overlay with additional read-only mounts.
05cd3ab to
f8ce5e3
Compare
Uh oh!
There was an error while loading. Please reload this page.